feat: Command Palette (⌘K) — chat actions framework#1323
Conversation
- Add PluginSettings(BaseModel) and SettingsField(hot=True) for typed plugin configuration with hot-update field markers - Add @quick_action decorator that works both above and below @plugin_entry - Add 'chat_command' to EntryKind literal type - Export PluginSettings, SettingsField, quick_action from plugin SDK surface - Refactor llm_tool import to avoid reload shadowing
- ActionAggregationService: collects actions from all providers
- ActionExecutionService: dispatches action execution (hot-config toggle,
plugin entry trigger, lifecycle control, navigation)
- ListActionsProvider: maps plugin list_actions metadata to descriptors
- SettingsActionProvider: auto-generates toggle/slider/dropdown actions
from PluginSettings hot=True fields
- SystemActionProvider: lifecycle start/stop/restart actions per plugin
- BuiltinActionsProvider: static built-in actions (e.g. open settings)
- PreferencesService: per-user action pinning/hiding persistence
- ActionDescriptor / UserActionPreferences domain models
- GET /actions, POST /actions/execute, GET/PUT /actions/preferences routes
- GET /plugin/{id}/settings/schema endpoint
- plugin_settings_resolver infrastructure helper
- Wire actions_router into http_app and routes/__init__
Add actions_proxy_router that forwards /api/actions/* requests from the main FastAPI app to the plugin server, enabling the React chat frontend to access the actions API without CORS issues. Endpoints proxied: - GET /api/actions → list available actions - POST /api/actions/execute → execute an action by id - GET /api/actions/preferences → get user action preferences - PUT /api/actions/preferences → update user action preferences
- CommandPalette.tsx: searchable action list with keyboard navigation, category grouping, parameter input forms, execution feedback - message-schema.ts: typed message protocol between host and React chat - App.tsx: mount CommandPalette, handle toggle via postMessage - styles.css: palette overlay, animations, dark/light theme support - app-react-chat-window.js: bridge postMessage events to React chat - Tests for CommandPalette rendering and message schema validation
- Integration: test_action_routes.py (full HTTP round-trip) - Unit: aggregation, execution, list_actions_provider, models, settings_provider, system_provider - Unit: actions_proxy_router (main server proxy)
# Conflicts: # plugin/sdk/plugin/__init__.py # plugin/sdk/plugin/settings.py # plugin/sdk/shared/core/decorators.py
…ginBase - get_user_language() / set_user_language() / fetch_user_language() - get_attachments() for image attachments from chat messages - export_push_image() for image export - Simplify data_path() to use config_dir / 'data' directly Fixes lifekit 'get_user_language' AttributeError after #1281 merge.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough新增端到端快速操作（Command Palette）：前端组件与样式、消息模式与宿主集成；主服 actions-proxy；插件服务器域模型、多个 ActionProvider、聚合/执行/偏好服务及路由；包含 SDK 与宿主增强与全面测试喵。 ChangesQuick Actions — 单一功能群
Estimated code review effort:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d6125a364
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const hasInlineWidget = item.type === 'instant' && ( | ||
| item.control === 'toggle' || item.control === 'entry_toggle' || | ||
| item.control === 'dropdown' || item.control === 'slider' || item.control === 'number' | ||
| ); |
There was a problem hiding this comment.
When a hot string setting is rendered, InlineWidget still places the text input in the row, but hasInlineWidget is false, so the row installs its button-style onKeyDown handler. While the input is focused, pressing Space bubbles to the row, which calls preventDefault(), so users cannot type spaces into text setting values. Include text in the inline-control set or stop propagation from the text input.
Useful? React with 👍 / 👎.
| updates: dict[str, object] = {toml_section: {field_name: value}} | ||
|
|
||
| result = await hot_update_plugin_config( | ||
| plugin_id=plugin_id, | ||
| updates=updates, | ||
| mode="temporary", |
There was a problem hiding this comment.
The execute route dispatches settings updates solely from the path, so any caller can craft plugin:settings:<field> and this code will send a temporary config update even when the plugin has no PluginSettings class or when <field> is not marked hot=True and was never returned by SettingsActionProvider. That bypasses the intended hot-field allowlist used to build the command palette; validate the resolved settings class and hot field before calling hot_update_plugin_config.
Useful? React with 👍 / 👎.
| plugin_id: str | None = None, | ||
| ) -> list[ActionDescriptor]: | ||
| """返回操作项列表。*plugin_id* 为 ``None`` 时返回所有。""" | ||
| ... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
plugin/sdk/plugin/base.py (1)
151-160: ⚡ Quick win
_last_attachments属性依赖过于隐式喵~这个方法在 line 159 回退到
getattr(self, "_last_attachments", None)，但是_last_attachments从来没有在__init__里初始化过喵。虽然用getattr加默认值是安全的，不会抛异常，但这种设计让人摸不着头脑喵！问题在于：
- 不知道什么时候、谁会设置
_last_attachments- 这个属性不是类接口的明确部分
- 依赖外部代码隐式设置属性容易出问题喵
建议要么在
__init__里初始化self._last_attachments = []，要么在文档字符串里说明这个属性的生命周期和设置时机喵~ 不...不是本喵看不懂啦，只是为了其他笨蛋开发者着想，哼！♻️ 建议的修复方案喵
在
__init__方法里初始化这个属性：def __init__(self, ctx): super().__init__(ctx) # Promote plugin-facing helper instead of exposing the shared minimal contract. from .runtime import Plugins self.plugins = Plugins(self.ctx) self._memory_client = None self._system_info_client = None self.i18n = self._load_plugin_i18n() self._static_ui_config: dict[str, Any] | None = None self._list_actions: list[dict[str, Any]] = [] + self._last_attachments: list[dict] = [] self._dynamic_entries: dict[str, dict[str, Any]] = {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/sdk/plugin/base.py` around lines 151 - 160, get_attachments falls back to an implicitly-set attribute _last_attachments which is never initialized; update the class __init__ to set self._last_attachments = [] (or an appropriate default) so the fallback is explicit and safe, and optionally add a short docstring/note on when/where _last_attachments is updated; locate the fix in the class constructor and ensure get_attachments, _last_attachments and __init__ are consistent.plugin/sdk/shared/runtime/system_info.py (1)
171-171: 💤 Low value确认返回空字符串的设计意图喵~
当配置中既没有
user_language_full也没有user_language时，这里返回Ok("")而不是Err，这意味着"未配置语言"被视为成功状态喵。调用方需要检查空字符串来判断是否真的获取到了语言设置喵。建议确认这个行为是否符合预期，并考虑在文档字符串中说明：返回空字符串表示未配置语言，这样其他开发者就不会误以为出错了喵~ 毕竟傲娇如本喵也会被这种微妙的设计搞迷糊呢，哼！
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/sdk/shared/runtime/system_info.py` at line 171, The function in plugin/sdk/shared/runtime/system_info.py currently returns Ok("") when neither user_language_full nor user_language is set; confirm this behavior is intentional (treating "no language configured" as success) and, if so, add/extend the docstring for the enclosing function (the one returning Ok("")) to state clearly that an empty string signals "language not configured" and callers must check for an empty value; if the intended behavior is instead to surface an error, change the return to an Err variant and update callers accordingly.plugin/server/domain/action_models.py (1)
24-40: ⚡ Quick win建议添加字段组合验证喵~
当前模型允许任意的可选字段组合,但某些
control类型需要特定的配套字段喵:
control="slider"应该要求min、max和step存在喵control="dropdown"应该要求options不为空喵control="number"可能也需要min/max范围喵control="toggle"应该有current_value喵可以考虑添加 Pydantic 的
@model_validator来强制这些约束,这样能在构造时就发现配置错误,而不是等到运行时才炸掉喵~💡 可选的验证器示例喵
+from pydantic import model_validator + class ActionDescriptor(BaseModel): """描述一个命令面板中的操作项。""" # ... existing fields ... + + `@model_validator`(mode="after") + def validate_control_fields(self) -> "ActionDescriptor": + """验证 control 类型与配套字段的一致性喵""" + if self.control == "slider": + if self.min is None or self.max is None or self.step is None: + raise ValueError("slider control requires min, max, and step") + elif self.control == "dropdown": + if not self.options: + raise ValueError("dropdown control requires non-empty options") + elif self.control == "number": + if self.min is None or self.max is None: + raise ValueError("number control requires min and max") + return self🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/server/domain/action_models.py` around lines 24 - 40, The model currently allows invalid field combinations for different control types; add a Pydantic model-level validator (e.g., a `@model_validator` on the class defined in plugin/server/domain/action_models.py that declares control, current_value, options, min, max, step, disabled) to enforce required fields per control: require min, max and step when control == "slider"; require non-empty options when control == "dropdown"; require min/max (or at least warn/validate) when control == "number"; and require current_value when control == "toggle"; the validator should inspect the control field and raise a ValueError with a clear message when the related fields are missing or invalid so invalid instances fail at construction.plugin/server/application/actions/system_provider.py (1)
223-231: ⚡ Quick win避免在导航动作里硬编码中文文案与关键词喵
这里把
label和keywords固定成中文，会让非中文语言环境下的命令面板搜索与理解变差，建议至少改为中立文案或按当前语言分支生成喵。♻️ 可参考修改喵
if _has_static_ui(meta): from config import USER_PLUGIN_SERVER_PORT as _ui_port actions.append(ActionDescriptor( action_id=f"system:{pid}:open_ui", type="navigation", - label=f"打开 {plugin_name} UI", + label=f"Open {plugin_name} UI", description="", category=plugin_name, plugin_id=pid, target=f"http://127.0.0.1:{_ui_port}/plugin/{pid}/ui/", open_in="new_tab", icon="↗", - keywords=[pid, plugin_name, "ui", "界面"], + keywords=[pid, plugin_name, "ui", "open"], ))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/server/application/actions/system_provider.py` around lines 223 - 231, The label and keywords for the navigation action are hardcoded in Chinese; update the code that builds the action (the dict using label and keywords with plugin_name and pid) to avoid fixed Chinese text by using a neutral/English default like "Open {plugin_name} UI" and English keywords (e.g., pid, plugin_name, "ui", "interface"), or wire it to your app i18n/locale function (e.g., gettext/t or current locale branch) so the label and keywords are generated per-language instead of being hardcoded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/sdk/plugin/base.py`:
- Line 114: 当前改动将 data_path() 改为返回 self.config_dir / "data" 导致与旧的
resolve_plugin_data_dir(self.ctx) 不一致并破坏现有插件数据访问；请恢复一致性：在 plugin/base.py 中将
data_path() 的实现改回使用 resolve_plugin_data_dir(self.ctx)（或若你决定统一到
self.config_dir，则同时把使用 resolve_plugin_data_dir 的 plugin_dir 赋值点改为使用
self.config_dir / "data" 并调整所有相关调用与测试），确保引用的唯一标识符为
resolve_plugin_data_dir、data_path、self.config_dir、plugin_dir 和 ctx，并在改动后运行并通过
plugin/tests/unit/sdk/plugin/test_sdk_v2_plugin_base.py 中期望的路径断言。
In `@plugin/server/application/actions/settings_provider.py`:
- Around line 273-279: When building current_value for hot fields in the loop
over settings_cls.model_fields (see _is_hot, field_info.annotation and
current_section usage), don't fall back to the raw sentinel field_info.default
because fields with a default_factory will return PydanticUndefined; instead
call field_info.get_default(call_default_factory=True) when current_section
lacks the key to invoke the factory and obtain a real default. Update the
assignment that sets current_value to use
field_info.get_default(call_default_factory=True) rather than
field_info.default, and add a unit test for a hot field with default_factory to
assert the returned current_value is JSON-serializable and has the expected
default.
---
Nitpick comments:
In `@plugin/sdk/plugin/base.py`:
- Around line 151-160: get_attachments falls back to an implicitly-set attribute
_last_attachments which is never initialized; update the class __init__ to set
self._last_attachments = [] (or an appropriate default) so the fallback is
explicit and safe, and optionally add a short docstring/note on when/where
_last_attachments is updated; locate the fix in the class constructor and ensure
get_attachments, _last_attachments and __init__ are consistent.
In `@plugin/sdk/shared/runtime/system_info.py`:
- Line 171: The function in plugin/sdk/shared/runtime/system_info.py currently
returns Ok("") when neither user_language_full nor user_language is set; confirm
this behavior is intentional (treating "no language configured" as success) and,
if so, add/extend the docstring for the enclosing function (the one returning
Ok("")) to state clearly that an empty string signals "language not configured"
and callers must check for an empty value; if the intended behavior is instead
to surface an error, change the return to an Err variant and update callers
accordingly.
In `@plugin/server/application/actions/system_provider.py`:
- Around line 223-231: The label and keywords for the navigation action are
hardcoded in Chinese; update the code that builds the action (the dict using
label and keywords with plugin_name and pid) to avoid fixed Chinese text by
using a neutral/English default like "Open {plugin_name} UI" and English
keywords (e.g., pid, plugin_name, "ui", "interface"), or wire it to your app
i18n/locale function (e.g., gettext/t or current locale branch) so the label and
keywords are generated per-language instead of being hardcoded.
In `@plugin/server/domain/action_models.py`:
- Around line 24-40: The model currently allows invalid field combinations for
different control types; add a Pydantic model-level validator (e.g., a
`@model_validator` on the class defined in plugin/server/domain/action_models.py
that declares control, current_value, options, min, max, step, disabled) to
enforce required fields per control: require min, max and step when control ==
"slider"; require non-empty options when control == "dropdown"; require min/max
(or at least warn/validate) when control == "number"; and require current_value
when control == "toggle"; the validator should inspect the control field and
raise a ValueError with a clear message when the related fields are missing or
invalid so invalid instances fail at construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1527a90c-d41b-42c0-b2af-00686136d5ce
📒 Files selected for processing (37)
app/main_server.pyfrontend/react-neko-chat/src/App.tsxfrontend/react-neko-chat/src/CommandPalette.tsxfrontend/react-neko-chat/src/message-schema.test.tsfrontend/react-neko-chat/src/message-schema.tsfrontend/react-neko-chat/src/styles.cssfrontend/react-neko-chat/src/test/CommandPalette.test.tsxmain_routers/actions_proxy_router.pyplugin/sdk/plugin/__init__.pyplugin/sdk/plugin/base.pyplugin/sdk/plugin/settings.pyplugin/sdk/shared/core/decorators.pyplugin/sdk/shared/runtime/system_info.pyplugin/server/application/actions/__init__.pyplugin/server/application/actions/aggregation_service.pyplugin/server/application/actions/builtin_provider.pyplugin/server/application/actions/execution_service.pyplugin/server/application/actions/list_actions_provider.pyplugin/server/application/actions/preferences_service.pyplugin/server/application/actions/settings_provider.pyplugin/server/application/actions/system_provider.pyplugin/server/domain/action_models.pyplugin/server/domain/action_provider.pyplugin/server/http_app.pyplugin/server/infrastructure/plugin_settings_resolver.pyplugin/server/routes/__init__.pyplugin/server/routes/actions.pyplugin/server/routes/config.pyplugin/tests/integration/test_action_routes.pyplugin/tests/unit/server/test_action_aggregation_service.pyplugin/tests/unit/server/test_action_execution_service.pyplugin/tests/unit/server/test_action_list_actions_provider.pyplugin/tests/unit/server/test_action_models.pyplugin/tests/unit/server/test_action_settings_provider.pyplugin/tests/unit/server/test_action_system_provider.pystatic/app-react-chat-window.jstests/unit/test_actions_proxy_router.py
- base.py: restore data_path() to use resolve_plugin_data_dir(self.ctx)
for backward compatibility with existing plugins
- base.py: initialize _last_attachments in __init__ for explicitness
- settings_provider.py: use field_info.get_default(call_default_factory=True)
instead of field_info.default to handle default_factory fields correctly
- system_info.py: clarify docstring that Ok('') means language not configured
- system_provider.py: replace hardcoded Chinese label/keywords with English
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/sdk/plugin/base.py`:
- Around line 153-163: The success path in get_attachments currently returns
result without updating the cache, so self._last_attachments remains stale;
modify get_attachments to assign the retrieved attachments into
self._last_attachments before returning: call result =
self.ctx.get_attachments(), and if result is not None then compute attachments =
list(result) if isinstance(result, list) else [], set self._last_attachments =
attachments, and then return attachments (preserving the try/except and fallback
behavior).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 041eb126-75b1-4013-9e14-5f9ecfc2fef1
📒 Files selected for processing (4)
plugin/sdk/plugin/base.pyplugin/sdk/shared/runtime/system_info.pyplugin/server/application/actions/settings_provider.pyplugin/server/application/actions/system_provider.py
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/sdk/shared/runtime/system_info.py
- plugin/server/application/actions/system_provider.py
- plugin/server/application/actions/settings_provider.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b3de9c33d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| effective = getattr(host, "get_effective_config", None) | ||
| if callable(effective): | ||
| cfg = effective() |
There was a problem hiding this comment.
When a hot setting is executed, ActionExecutionService applies it with mode="temporary", but the subsequent action refresh reaches this branch and then falls back to load_plugin_config because no current host implementation exposes get_effective_config (repo-wide rg get_effective_config only finds this call). For any temporary command-palette setting change, the refreshed descriptor therefore reads the persisted TOML value instead of the value just applied in the running plugin, so toggles/sliders snap back to stale values and the next click can send the wrong value again. The provider needs a real source for the child process's effective config or another cache of temporary updates.
Useful? React with 👍 / 👎.
- CommandPalette.tsx: include 'text' in hasInlineWidget set so space key is not intercepted by the row's onKeyDown handler - execution_service.py: validate that the target field is actually a hot=True field before dispatching settings update (prevents bypassing the hot-field allowlist via crafted action_id) - settings_provider.py: document the known limitation that temporary hot-updates may show stale TOML values until host.get_effective_config is implemented; frontend uses ActionExecuteResponse.action as workaround
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b9fc1f95b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| static_ui_obj = meta.get("static_ui_config") | ||
| if not isinstance(static_ui_obj, Mapping): | ||
| return False |
There was a problem hiding this comment.
For plugins that only provide <plugin dir>/static/index.html and rely on the existing ui_query_service config_path inference, this branch returns false because it requires an explicit static_ui_config. The /plugin/{id}/ui/ route can still serve those UIs via plugin/server/application/plugins/ui_query_service.py, but the command palette will omit the Open UI action; resolve the static UI config through the same helper before checking the directory.
Useful? React with 👍 / 👎.
| current_value=str(current_value) if current_value is not None else None, | ||
| options=[str(a) for a in args], |
There was a problem hiding this comment.
When a hot setting is annotated as a non-string Literal such as Literal[1, 2], the descriptor stringifies both the current value and options, and the frontend dropdown posts that string back unchanged. The config hot-update then validates against the original PluginSettings type, so selecting "1" for a Literal[1, 2] field is rejected even though the provider exposed it as editable; keep enough type information to coerce the submitted value back to the literal's original type.
Useful? React with 👍 / 👎.
- base.py: update _last_attachments cache on success path in get_attachments - base.py: add explanatory comments to empty except blocks - test_action_execution_service.py: remove unused ActionExecuteResponse import - test_action_list_actions_provider.py: remove unused threading import - test_action_routes.py: remove unused Any import, unify module import style - test_action_system_provider.py: remove unused Mapping import
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/sdk/plugin/base.py`:
- Around line 157-159: The method currently assigns a list to
self._last_attachments and returns that same list reference (see the if
isinstance(result, list) block and the _last_attachments attribute); change the
return to provide a shallow copy instead of the internal list to avoid exposing
mutable internal state—i.e., after setting self._last_attachments =
list(result), return a copy (e.g., list(self._last_attachments) or
self._last_attachments.copy()) so callers cannot mutate the internal cache.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b431e163-229f-4517-a764-f359766ef8dc
📒 Files selected for processing (5)
plugin/sdk/plugin/base.pyplugin/tests/integration/test_action_routes.pyplugin/tests/unit/server/test_action_execution_service.pyplugin/tests/unit/server/test_action_list_actions_provider.pyplugin/tests/unit/server/test_action_system_provider.py
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/tests/unit/server/test_action_system_provider.py
- plugin/tests/unit/server/test_action_execution_service.py
- plugin/tests/unit/server/test_action_list_actions_provider.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df806d404e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| label = str(action.get("label") or action_id) | ||
| description = str(action.get("description") or "") |
There was a problem hiding this comment.
When a plugin defines list_actions.label or description as an i18n mapping, which the existing plugin UI query path accepts and resolves via resolve_i18n_refs, this coerces the mapping to a Python dict string (for example {'en-US': 'Open'}) before sending it to the command palette. Those actions then display and search against the raw repr instead of the localized label; resolve list action metadata through the same i18n helper before building descriptors.
Useful? React with 👍 / 👎.
| monkeypatch.setattr( | ||
| "plugin.server.application.actions.execution_service.resolve_settings_class", | ||
| lambda pid: None, | ||
| ) |
There was a problem hiding this comment.
This test patches resolve_settings_class to return None, but _SettingsActionHandler.execute now validates that a settings class exists and will raise SETTINGS_NOT_FOUND before calling hot_update_plugin_config. With the test dependencies installed, svc.execute(...) fails here instead of reaching the assertions; use a fake PluginSettings class with a hot field, or change this test to assert the new rejection behavior.
Useful? React with 👍 / 👎.
- base.py: return copy of _last_attachments to prevent cache mutation - base.py: add comment to fetch_user_language except block - preferences_service.py: add comment to temp file cleanup except block - test_action_execution_service.py: provide fake PluginSettings with hot field to match new validation in _SettingsActionHandler.execute
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0143e3a3cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| action_id, | ||
| ) | ||
| return None | ||
| target = raw_target.strip() |
There was a problem hiding this comment.
For static list_actions that use the supported {plugin_id} placeholder (the existing plugin UI query path replaces it in plugin/server/application/plugins/ui_query_service.py:481), this provider sends the raw target to the command palette. In that case clicking the command navigates to a literal URL like /plugins/{plugin_id}?tab=panel or https://example.com/{plugin_id} instead of the plugin-specific route, even though the same action works from the plugin list UI; reuse the same normalization before building the descriptor.
Useful? React with 👍 / 👎.
- list_actions_provider: resolve plugin i18n refs for label/description
and the plugin name itself, mirroring system_provider behavior so
$i18n mappings no longer surface as raw dict reprs in the palette.
- list_actions_provider: substitute the `{plugin_id}` placeholder in
navigation / chat_inject targets, mirroring the normalization that
ui_query_service already applies for the plugin list UI.
- system_provider: replace the local _has_static_ui with a delegate to
ui_query_service._has_static_ui_from_meta so plugins relying on the
conventional <plugin dir>/static/index.html inference (no explicit
static_ui_config) are recognized — matches what /plugin/{id}/ui/
actually serves.
- test_action_execution_service: drop legacy `class Config: pass` from
_FakeSettings (pydantic v2 rejects `Config` + `model_config` together,
introduced in the third-round fix).
- tests: cover plugin_id substitution, $i18n label resolution, and the
inferred-static-UI path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/server/application/actions/list_actions_provider.py`:
- Around line 76-85: The code checks action_id with strip() but uses the
original value when building full_action_id which can keep surrounding
whitespace; normalize and reuse a cleaned action_id instead: coerce
action.get("id") to a string, call .strip() (and optionally .lower() if you want
case-normalization) into a new variable (e.g., cleaned_action_id or reuse
action_id after assignment), validate that cleaned value is non-empty, and build
full_action_id = f"{plugin_id}:{cleaned_action_id}" (update any subsequent uses
to reference the cleaned/normalized identifier).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 616f48f5-3a2d-4525-a2f0-54688d512290
📒 Files selected for processing (5)
plugin/server/application/actions/list_actions_provider.pyplugin/server/application/actions/system_provider.pyplugin/tests/unit/server/test_action_execution_service.pyplugin/tests/unit/server/test_action_list_actions_provider.pyplugin/tests/unit/server/test_action_system_provider.py
✅ Files skipped from review due to trivial changes (1)
- plugin/tests/unit/server/test_action_list_actions_provider.py
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/server/application/actions/system_provider.py
- plugin/tests/unit/server/test_action_system_provider.py
- plugin/tests/unit/server/test_action_execution_service.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a0bcb9641
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception as exc: | ||
| raise ServerDomainError( | ||
| code="ENTRY_TRIGGER_FAILED", | ||
| message=f"Failed to trigger entry '{entry_id}': {exc}", | ||
| status_code=500, |
There was a problem hiding this comment.
The broad except Exception here wraps intentionally raised ServerDomainErrors (for example ENTRY_TRIGGER_UNSUPPORTED/ENTRY_TOGGLE_UNSUPPORTED) into a generic 500 ENTRY_*_FAILED, which changes expected status codes and hides actionable error codes from clients. This makes unsupported host capabilities look like internal failures; ServerDomainError should be allowed to propagate unchanged before the generic fallback wrapping.
Useful? React with 👍 / 👎.
| handler = self._DISPATCH.get(action) | ||
| if handler is not None: | ||
| return await handler(self, plugin_id, action_id, value) |
There was a problem hiding this comment.
Lifecycle dispatch accepts any system:{plugin_id}:{action}:... string because it only checks parts[2], so malformed IDs like system:demo:stop:unexpected still execute privileged lifecycle operations. The route should require exactly three segments for lifecycle actions and reserve extra segments only for the entry form to avoid executing unintended actions from malformed/crafted IDs.
Useful? React with 👍 / 👎.
| continue | ||
| meta: dict[str, Any] = dict(meta_raw) | ||
|
|
||
| plugin_name = str(meta.get("name") or pid) |
There was a problem hiding this comment.
Settings actions derive plugin_name with str(meta.get("name")), so plugins that provide i18n name mappings render categories/keywords as raw dict strings instead of localized text. Other action providers in this change already resolve i18n metadata, so this creates an inconsistent UX where settings entries for localized plugins are mislabeled and harder to search.
Useful? React with 👍 / 👎.
- execution_service: re-raise ServerDomainError before the broad except in entry button/toggle paths so intentional ENTRY_TRIGGER_UNSUPPORTED / ENTRY_TOGGLE_UNSUPPORTED (501) are not collapsed into a generic ENTRY_*_FAILED (500), which had hidden the actionable code and status. - execution_service: lifecycle dispatch (start/stop/reload/toggle/profile) now requires exactly 3 segments — crafted ids like `system:demo:stop:unexpected` no longer reach the privileged op. - settings_provider: resolve plugin name through plugin i18n before building descriptors, matching list_actions_provider and system_provider so $i18n refs no longer leak as dict reprs into the command palette category / keywords. - list_actions_provider: normalize action_id with strip() before joining with plugin_id so a raw " foo " no longer produces "demo: foo " ids. - tests: cover the unsupported-error preservation, lifecycle segment rejection, settings-provider i18n resolution, and id whitespace normalization. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7144b1f2cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try: | ||
| base = await _resolve_user_plugin_base() | ||
| async with httpx.AsyncClient(timeout=10.0, proxy=None, trust_env=False) as client: | ||
| resp = await client.post(f"{base}/chat/actions/{action_id}/execute", json=body) |
There was a problem hiding this comment.
proxy_chat_action_execute forwards action_id by direct string interpolation, so reserved URL characters in IDs (e.g. ?, #, %) change the outgoing request path/query and the plugin server can receive a different action ID or a 404. This is reachable because action IDs are plugin-defined and not restricted to URL-safe characters (the list-action provider only enforces non-empty strings), so a valid configured action can become unexecutable through the main-server proxy.
Useful? React with 👍 / 👎.
| if not isinstance(meta_raw, Mapping): | ||
| continue | ||
| meta: dict[str, Any] = dict(meta_raw) | ||
| plugin_name = str(meta.get("name") or pid) |
There was a problem hiding this comment.
Builtin lifecycle actions derive plugin_name with str(meta.get("name") or pid), which turns i18n name mappings into raw dict strings (for example {'$i18n': ...}) in labels and keywords. In that case command names become unreadable and search matching degrades for localized plugins, unlike other action providers in this change that resolve i18n metadata before rendering descriptors.
Useful? React with 👍 / 👎.
- actions_proxy_router: percent-encode action_id before interpolating it
into the outgoing /chat/actions/{action_id}/execute path. Reserved URL
chars (`?`, `#`, `%`, `/`) in plugin-defined IDs would otherwise
reinterpret the request as path+query/fragment and reach the plugin
server as a different (or 404) action. `:` stays unencoded so the
canonical `system:demo:toggle` form remains readable.
- builtin_provider: resolve plugin name through plugin i18n (matching
list_actions, system, and settings providers from the earlier rounds)
so `$i18n` refs no longer leak as raw dict reprs into lifecycle action
labels and keywords.
- tests: cover URL encoding for `?` / `#` / `%` while preserving `:`,
and add a dedicated builtin_provider test module covering lifecycle
emission plus i18n ref resolution.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| try: | ||
| response = await client.get(f"{base}/available") | ||
| if response.is_success: | ||
| _USER_PLUGIN_BASE_CACHE = (base, now) |
| continue | ||
|
|
||
| fallback = str(USER_PLUGIN_BASE or _USER_PLUGIN_DEFAULT_BASE).rstrip("/") | ||
| _USER_PLUGIN_BASE_CACHE = (fallback, now) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/tests/unit/server/test_action_builtin_provider.py`:
- Around line 89-90: The test currently uses all(a.plugin_id == "b" for a in
actions) which will pass on an empty list; update the assertion to prevent false
positives by first asserting actions is non-empty (e.g., assert actions) and
then keeping the all(...) check, or replace both with a precise check of the
returned action IDs (e.g., compare {a.action_id for a in actions} to the
expected set). Apply this change around the call to _collect and references to
actions, plugin_id and action_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2ab289e6-7271-4a2a-9dcd-1c5d4082c67c
📒 Files selected for processing (4)
main_routers/actions_proxy_router.pyplugin/server/application/actions/builtin_provider.pyplugin/tests/unit/server/test_action_builtin_provider.pytests/unit/test_actions_proxy_router.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/unit/test_actions_proxy_router.py
- plugin/server/application/actions/builtin_provider.py
- main_routers/actions_proxy_router.py
`all(a.plugin_id == "b" for a in actions)` returns True on `actions == []`, so a regression that drops the filter output entirely would still pass. Add a non-empty guard before the all() check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb505a375c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if action_id.startswith("system:"): | ||
| return await self._system_handler.execute(action_id, value) |
There was a problem hiding this comment.
system plugin IDs before dispatching by prefix
ActionExecutionService.execute routes every action whose ID starts with system: to the lifecycle handler, so any plugin with plugin_id == "system" will have all of its settings/entry action IDs misclassified and become unexecutable (or target the wrong lifecycle path). Because dispatch is purely prefix-based here, this introduces a hard collision with a valid plugin namespace and breaks command palette actions for that plugin.
Useful? React with 👍 / 👎.
| item={item} | ||
| loading={!!loadingMap[item.action_id]} | ||
| error={errorMap[item.action_id] ?? null} | ||
| highlighted={false} |
There was a problem hiding this comment.
Keyboard navigation is wired to activate the highlighted row (.cp-row-highlighted.cp-row-clickable) on Enter, but grouped rendering forces every row to highlighted={false}. In "按插件" view, ArrowUp/ArrowDown updates highlightIdx but no row ever receives the highlight class, so Enter cannot execute any command via keyboard.
Useful? React with 👍 / 👎.
- execution_service: split dispatch by structure rather than the bare
`system:` prefix, so a plugin whose `plugin_id == "system"` is no
longer a tombstone. The new order:
1. `{plugin_id}:settings:{field}` (wins, even for plugin "system")
2. `system:{plugin_id}:{lifecycle|entry...}` ONLY when parts[2] is a
known lifecycle keyword (start/stop/reload/toggle/profile/entry)
3. `{plugin_id}:{action_id}` fall-through to list-action handler.
Existing tests that asserted ACTION_NOT_FOUND from the system handler
for `system:bad` / `system:demo:explode` now correctly land in the
list-action handler (ACTION_NOT_IMPLEMENTED) — the system handler is
no longer reserved for ambiguous strings.
- CommandPalette: pipe `highlightedActionId` from CommandPalette into
PluginCard so grouped view rows actually receive the highlight class.
PluginCard auto-expands while one of its rows is highlighted so
keyboard Enter resolves a visible `.cp-row-highlighted.cp-row-clickable`
(the parent's Enter handler activates via querySelector + .click()).
- tests: cover plugin "system" list/settings routing (including the
`system:settings:start` collision case where the field name is also a
lifecycle keyword), update the two stale system-handler tests, and add
a Vitest case asserting Enter activation works in `byPlugin` view.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL's `Superfluous trailing arguments` rule infers the constructor signature from this stub and has been re-firing on every push to flag `new ResizeObserver(measure)` in CommandPalette.tsx — even though that matches the real DOM API which takes a `ResizeObserverCallback`. Give the stub an optional `cb?: ResizeObserverCallback` parameter so its shape matches the real type, silencing the false positive without touching production code. The stub still no-ops; the cb is never invoked, which is fine for the jsdom test environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00f9d53807
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (prop?.type === 'number' || prop?.type === 'integer') { | ||
| args[key] = Number(raw) || 0; | ||
| } else if (prop?.type === 'boolean') { | ||
| args[key] = raw === 'true' || raw === '1'; | ||
| } else { | ||
| args[key] = raw; |
There was a problem hiding this comment.
ParamForm.submit always emits every schema property and coerces blank inputs to concrete values (Number(raw) || 0, false, or ""). For entries with optional parameters, submitting without filling a field no longer preserves server/plugin defaults; it actively overrides them with 0/false/empty string, which can change behavior or trigger validation errors in handlers expecting missing fields to be omitted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugin/tests/unit/server/test_action_execution_service.py (1)
411-415: ⚡ Quick win这两个回归用例还缺少对
resp.action的断言喵现在它们只验证了 hot-update 的入参有没有走对，但前面那个 namespace bug 真正坏掉的是返回载荷里的
ActionExecuteResponse.action。也就是说，哪怕resp.action is None，这两条测试现在还是会绿掉，兜不住这次回归喵。建议这里给
_build_service()注入一个带目标ActionDescriptor的_FakeAggregationService，再补一条assert resp.action is not None/assert resp.action.action_id == "system:settings:..."，这样才能把这个场景真正锁住喵。Also applies to: 446-450
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/tests/unit/server/test_action_execution_service.py` around lines 411 - 415, 测试目前只验证了 hot-update 的入参，但没有断言返回载荷里的 ActionExecuteResponse.action，这会漏掉 namespace 相关的回归；在调用 _build_service() 时注入一个返回带目标 ActionDescriptor 的 _FakeAggregationService（保证执行时会填充 resp.action），并在原有断言后补上 assert resp.action is not None 以及 assert resp.action.action_id == "system:settings:enabled"（或与该用例对应的 action_id），以确保 resp.action 被正确返回并锁住该回归场景。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/server/application/actions/execution_service.py`:
- Around line 50-53: The current _find_action() misparses action_id like
"system:settings:enabled" as a system lifecycle action and sets plugin_id to
"settings", causing aggregate_actions(plugin_id=...) to miss the updated
descriptor and ActionExecuteResponse.action to be None; fix by changing the
parsing logic in _find_action(): when action_id.startswith("system:") split into
parts but only treat it as a system lifecycle action if parts[1] is not
"settings" (i.e., check parts[1] != "settings" before assigning plugin_id =
parts[1]), or better yet modify _find_action() signature to accept an explicit
plugin_id parameter (and prefer that over guessing) so
aggregate_actions(plugin_id=...) uses the correct plugin id and the action
lookup returns the updated descriptor for ActionExecuteResponse.action.
---
Nitpick comments:
In `@plugin/tests/unit/server/test_action_execution_service.py`:
- Around line 411-415: 测试目前只验证了 hot-update 的入参，但没有断言返回载荷里的
ActionExecuteResponse.action，这会漏掉 namespace 相关的回归；在调用 _build_service()
时注入一个返回带目标 ActionDescriptor 的 _FakeAggregationService（保证执行时会填充
resp.action），并在原有断言后补上 assert resp.action is not None 以及 assert
resp.action.action_id == "system:settings:enabled"（或与该用例对应的 action_id），以确保
resp.action 被正确返回并锁住该回归场景。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7e7425c6-019b-4223-ba3d-c65815b59bed
📒 Files selected for processing (5)
frontend/react-neko-chat/src/CommandPalette.tsxfrontend/react-neko-chat/src/test/CommandPalette.test.tsxfrontend/react-neko-chat/src/test/setup.tsplugin/server/application/actions/execution_service.pyplugin/tests/unit/server/test_action_execution_service.py
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/react-neko-chat/src/test/CommandPalette.test.tsx
- frontend/react-neko-chat/src/CommandPalette.tsx
Two findings landed nearly simultaneously and share the same theme as the previous round (preserve caller intent / don't override defaults). - ParamForm.submit (CommandPalette.tsx): stop coercing untouched optional parameter fields to ``0`` / ``false`` / ``""``. Skip blank inputs so the server/plugin can apply its own default; numeric strings that fail to parse are also skipped. Filled fields are still emitted verbatim. - execution_service._find_action: ``system:`` prefix dispatch had the same bug previously fixed in ``execute()`` — it always read ``parts[1]`` as plugin_id, which silently misclassified plugin "system" settings (e.g. ``system:settings:enabled`` looked like a "settings" plugin lifecycle action) and caused the re-fetched descriptor to come back ``None``. Extracted the structural mapping into a shared ``_plugin_id_from_action_id`` helper that mirrors ``execute()`` and promoted ``_SYSTEM_LIFECYCLE_KEYS`` to module level so dispatch and reverse lookup stay aligned. - tests: ParamForm omit-blank and verbatim-emit cases; reverse-lookup cases for lifecycle / plugin "system" settings / normal plugin settings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…se lookup
Two threads from this round of review:
- coderabbit nitpick: the previous regression tests for plugin "system"
only asserted on hot_update arguments, not on the returned
ActionExecuteResponse.action. If _find_action had still been buggy,
resp.action would silently be None and the tests would still pass.
Inject a _FakeAggregationService carrying the matching descriptor and
assert resp.action.action_id, locking the response payload.
- Fix found while writing the assertion: _plugin_id_from_action_id had
the same settings-vs-lifecycle precedence bug that execute() had —
``system:settings:start`` (field name collides with a lifecycle
keyword) fell into the lifecycle shortcut and returned "settings" as
plugin_id. Check the {pid}:settings:{field} structural form before the
lifecycle shortcut, mirroring execute(). Added a dedicated precedence
case to TestPluginIdFromActionId.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/react-neko-chat/src/test/CommandPalette.test.tsx`:
- Around line 222-229: The test currently only checks visibility for the hidden
item 'demo:greet' but must also assert its "greyed" state; after rendering via
renderPalette(allItems, prefs) and typing into the input (placeholder
'搜索操作...'), locate the 'Greet' item (screen.getByText('Greet')) and assert it
has the dimmed CSS class used by the palette (e.g., palette-item--dimmed) or the
accessible state (e.g., aria-disabled="true") so the test verifies the
visual/semantic "greyed" behavior for hidden items as intended.
- Line 262: Replace the DOM-traversal click that uses label.closest('.cp-row')
in the test with an accessible query: use screen.getByRole('button', { name:
<visible label text> }) and pass that node to fireEvent.click; specifically,
locate where fireEvent.click(label.closest('.cp-row')!) is used in
CommandPalette.test and change it to fireEvent.click(screen.getByRole('button',
{ name: '...' })) (use the actual button label text from the test) so the test
relies on role/name queries instead of the .cp-row class.
In `@plugin/server/application/actions/execution_service.py`:
- Around line 419-433: The current _ListActionHandler.execute unconditionally
raises ACTION_NOT_IMPLEMENTED, preventing any {plugin_id}:{action_id} list
actions from running; replace the hard-coded ServerDomainError with a call that
dispatches execution to the actual list action provider (e.g., use the
ListActionsProvider instance or the provider method that executes actions such
as execute_action/handle_action) passing action_id and value, await its response
and return an ActionExecuteResponse; propagate provider errors as appropriate
(map to ACTION_EXECUTION_FAILED with details) instead of 501, and apply the same
fix to the equivalent handler code referenced around lines 488-491 so list
actions are executed rather than always returning not-implemented.
- Around line 293-305: After calling
set_plugin_active_profile()/set_plugin_active_profile (the profile change), if
await self._lifecycle.reload_plugin(plugin_id) raises, do not return an
unqualified success; catch the exception, log the full error (keep
logger.warning but include exc), and return an ActionExecuteResponse that
clearly indicates a partial failure (e.g., success=False or a dedicated partial
status) with a message like "Profile updated to '<profile_name>' but reload
failed for plugin <plugin_id>; plugin will not use the new profile until manual
restart/reload" so callers/front-end aren’t misled—update the except block
around reload_plugin to construct and return that ActionExecuteResponse instead
of falling through to the full-success return.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 59d15e36-e95d-4335-b566-bea14d220ee0
📒 Files selected for processing (4)
frontend/react-neko-chat/src/CommandPalette.tsxfrontend/react-neko-chat/src/test/CommandPalette.test.tsxplugin/server/application/actions/execution_service.pyplugin/tests/unit/server/test_action_execution_service.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin/tests/unit/server/test_action_execution_service.py
- frontend/react-neko-chat/src/CommandPalette.tsx
- execution_service._profile: when set_plugin_active_profile succeeds but reload_plugin fails, the config DID change but the running plugin is still on the old profile. The previous code returned a misleading "Profile switched" success message; now the message surfaces the reload failure and tells the user a manual reload is needed. success stays True because the persisted profile did change. - CommandPalette.test.tsx: the 'hidden items (greyed) when searching' case only asserted visibility, so a style regression that drops the greyed treatment would have left it green. Lock the is-hidden class on the row wrap to actually cover the visual contract. - tests: add a reload-failure regression test that asserts the message surfaces both the profile name and the reload error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL flagged the dead inner function — I'd left an unused async stub above the actual lambda patch. Production calls ``set_plugin_active_profile`` via ``asyncio.to_thread``, so the patch needs a sync callable anyway; the lambda is correct, the async def was just leftover scaffolding. Comment now explains the sync requirement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
我只想知道 肘击完还能用吗现在 |
- Match the ⌘ glyph color to the toolbar's #44b7fe blue - Add hover opacity transition and active state background - Dark mode active state matches emoji button pattern
…-O/N.E.K.O into topic/chat-actions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9b172712e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| // "按插件" mode: show all, no filter tabs | ||
| if (viewMode === 'byPlugin') { | ||
| const sorted = [...visible].sort(sortByPriority); |
There was a problem hiding this comment.
In byPlugin view with items from multiple plugins whose global priority/label sort interleaves them (for example A1, B1, A2), displayItems remains in that global order while renderGrouped clusters rows by plugin. Since ArrowUp/ArrowDown indexes displayItems but the DOM order is grouped, the highlight can jump to a non-adjacent visual row and Enter can activate a row that is not the next one users see; build displayItems in the same grouped order used for rendering or drive navigation from the rendered row order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a231cc3. In byPlugin mode, non-search displayItems are now flattened from the same plugin-grouped order used by rendering, while search keeps the existing flat sorted order. Added a regression test for interleaved cross-plugin priorities and verified with npm test -- CommandPalette.test.tsx plus npm run typecheck.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a231cc3be7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| description="", | ||
| category=plugin_name, | ||
| plugin_id=pid, | ||
| target=f"http://127.0.0.1:{_ui_port}/plugin/{pid}/ui/", |
There was a problem hiding this comment.
The Open UI action hardcodes http://127.0.0.1:<port>/plugin/{id}/ui/, which only works when the browser is running on the same machine as the plugin server. In remote/LAN access, the user’s browser resolves 127.0.0.1 to its own device, so clicking the command opens a dead local URL instead of the server plugin UI. This breaks command-palette UI navigation for non-local clients; construct the URL from the current host (or a proxied route) rather than fixed loopback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0b36633. The quick-action navigation path now rewrites loopback plugin UI URLs to the current page host when the chat window is being accessed from a remote/LAN host, while preserving the plugin server port and leaving local/non-plugin targets unchanged. Added App-level regression coverage for the host rewrite and unchanged local cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b36633012
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception as exc: | ||
| raise ServerDomainError( | ||
| code="PLUGIN_PROFILE_ACTIVATE_FAILED", | ||
| message=f"Failed to set active profile '{profile_name}'", | ||
| status_code=500, |
There was a problem hiding this comment.
set_plugin_active_profile can raise client-facing errors (for example, missing profiles are raised as 404), but this broad except Exception rewrites all of them to PLUGIN_PROFILE_ACTIVATE_FAILED with status 500. In that scenario, callers receive an internal-error response for invalid input, which breaks actionable UX and error handling; preserve known exception status/code paths before the generic fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d5495ea. Profile activation now lets existing ServerDomainError paths propagate before the generic fallback, so client-facing errors such as missing profiles keep their original code/status instead of being rewritten to PLUGIN_PROFILE_ACTIVATE_FAILED/500. Added a regression test for the 404 preservation path.
| icon=str(qa_icon) if qa_icon else "⚡", | ||
| keywords=[pid, plugin_name, str(entry_name)], | ||
| quick_action=bool(is_quick), | ||
| priority=int(qa_priority) if is_quick else 0, |
There was a problem hiding this comment.
priority=int(qa_priority) can raise ValueError when plugin metadata carries a non-numeric priority (e.g., mis-typed decorator input or mutated metadata). That exception aborts SystemActionProvider, and because aggregation catches failures per provider, one bad entry removes all system actions from the response instead of degrading just that item; use safe coercion with a fallback per action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d5495ea. Quick-action priority coercion now falls back to 0 on TypeError/ValueError, so malformed plugin metadata cannot abort the whole SystemActionProvider collection. Added a regression test covering a non-numeric quick_action priority.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4afbf0c565
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| continue | ||
| descriptor = _map_list_action(pid, plugin_name, resolved_action) | ||
| if descriptor is not None: | ||
| actions.append(descriptor) |
There was a problem hiding this comment.
When a plugin's metadata contains duplicate list_actions IDs (possible via static metadata or set_list_actions()), this provider appends both descriptors with the same action_id, while the existing plugin-list UI path deduplicates IDs before returning actions. In the command palette those duplicate IDs are used as React keys and preference/recent identifiers, so hiding or pinning one action also affects the other and rendering can become unstable; keep a per-plugin seen set before appending descriptors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbb1826f3f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return await self.ctx.export_push(**kwargs) | ||
|
|
||
| async def export_push_image(self, **kwargs: Any) -> object: | ||
| return await self.ctx.export_push_image(**kwargs) |
There was a problem hiding this comment.
When plugin authors call the newly exposed await self.export_push_image(...), this forwards to self.ctx.export_push_image, but the context classes exposed in this repo only provide the generic export_push/binary helpers (rg export_push_image only finds this wrapper). In any plugin using this API, it raises AttributeError instead of exporting the image; either add the context method or route this wrapper through the existing export/push-message API.
Useful? React with 👍 / 👎.
| def get_attachments(self) -> list[dict]: | ||
| """Return image attachments forwarded from the user's chat message.""" | ||
| try: | ||
| result = self.ctx.get_attachments() |
There was a problem hiding this comment.
For plugins that try to read image attachments via the new get_attachments() helper, this calls self.ctx.get_attachments(), but rg get_attachments shows no context/host implementation and _last_attachments is only populated after that call succeeds. As a result the helper always falls back to the initial empty cache, so plugins cannot access forwarded chat attachments even when the user sent images.
Useful? React with 👍 / 👎.
2 participants
Summary
Add a unified Command Palette (⌘K) to the chat window, allowing users to search and execute plugin actions without leaving the conversation.
Changes
SDK enhancements
PluginSettingsbase class: callablejson_schema_extrasupport,__neko_hot__metadata marker for robust hot-field detection@quick_actiondecorator: bidirectional compatibility (works above or below@plugin_entry)NekoPluginBase: addget_user_language()/fetch_user_language()/get_attachments()/export_push_image()(fixes feat: add lifekit plugin #1281 missing methods)SystemInfo.get_user_language(): reuses existingget_system_config()infrastructureActions backend (plugin-server)
ActionAggregationService: collects actions from all providersSettingsActionProvider: auto-generates toggle/slider/dropdown actions fromPluginSettingshot fieldsListActionsProvider: maps pluginlist_actionsmetadata to descriptorsSystemActionProvider: entry buttons + static UI navigation per pluginBuiltinActionsProvider: static built-in actionsActionExecutionService: dispatches execution (hot-config update viahot_update_plugin_config, entry trigger, lifecycle viaPluginLifecycleService)PreferencesService: per-user pin/hide/recent persistenceGET /chat/actions,POST /chat/actions/{id}/execute,GET/POST /chat/actions/preferencesGET /plugin/{id}/settings/schemaendpointMain-server proxy
actions_proxy_router: forwards/api/actions/*from main FastAPI app to plugin-server (required because React chat iframe can only reach the main port)Frontend (React chat)
CommandPalette.tsx: searchable action list, keyboard navigation, inline controls (toggle/slider/dropdown/number/text/button), category grouping, pin/hide context menumessage-schema.ts: typed postMessage protocol between host page and React chatapp-react-chat-window.js: bridge ⌘K shortcut and action data to the iframeTests
Architecture notes
hot_update_plugin_config,PluginLifecycleService,state.plugin_hosts,push_message v2,SystemInfo.get_system_config()system_routerforwarding to memory_serverpytest-asyncio(CI has it)Summary by CodeRabbit